Define messaging metrics and add error.type attribute to spans#163
Define messaging metrics and add error.type attribute to spans#163joaopgrassi merged 8 commits intoopen-telemetry:mainfrom
error.type attribute to spans#163Conversation
cdbdf57 to
211bf76
Compare
pyohannes
left a comment
There was a problem hiding this comment.
Thanks for putting a stake in the ground here, this is a great start.
11af3e7 to
4dcbade
Compare
6bdad37 to
fec893f
Compare
|
I love the duration metrics. Did we consider stronger consistency with existing
|
On the messaging side, the current metric names relate to the messaging specific operation names. For the consumer side I definitely see value in having separate consumer metrics pull-based (receive) and push-based (deliver) scenarios. The duration of pull and push durations aren't semantically consistent, as the push duration usually also involves the duration of processing the message, whereas the pull duration doesn't. We shouldn't mix both in one single metric. |
fec893f to
2baa64d
Compare
2baa64d to
034ff58
Compare
034ff58 to
d4aefb4
Compare
a1ec11b to
adc9624
Compare
adc9624 to
6ae1dab
Compare
error.type attribute to spans
|
For spans, messaging systems add system-specific attributes to the spans. Do we want to treat messaging system specific metric dimensions the same way, so that messaging systems extend existing metrics? Or do we require them to send different system-specific metrics? The first approach makes cardinality hard to control (imagining a single service using two different messaging systems), the second one will likely duplicate information. |
5b1cc90 to
93112d0
Compare
I think there a couple of options here:
I suggest to start with Option 1 - as messaging semconv progresses towards stability, we should get more feedback from messaging systems and instrumentation prototypes and can change the approach. |
pyohannes
left a comment
There was a problem hiding this comment.
There are still some details to clarify, but this provides a great starting point.
joaopgrassi
left a comment
There was a problem hiding this comment.
Overall looks like a good start! Left some non blocking comments.
93112d0 to
14c18bf
Compare
|
@open-telemetry/specs-semconv-approvers this PR is approved by the messaging WG members, please take a look |
Fixes open-telemetry/opentelemetry-specification#1014